-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add minimal viable docs for addons #11444
Conversation
@ericholscher @humitos this is a pretty minimal start. If you're happy with the direction, I'll do a little more cleanup and we can merge this. Are we considering the Sponsorship thing as an addon? |
I've added this PR to my list. I will try to review it soon.
It's implemented as an addon. However, it doesn't really add too much value to the user, so I'm not sure how to talk about this one in particular. We have to mention it somehow, but maybe what we already have is enough since the user doesn't care about the implementation: https://docs.readthedocs.io/en/stable/advertising/ethical-advertising.html |
Yeah, that tracks. I'd suggest removing it from this 'Addons' list in that case. |
@humitos when you take a look, keep an eye out for any places that need more detail in the difference between the new and old feature, like in search. Also, the advertising section is pretty hidden in the RTD docs, I linked to Sponsorship instead, because that's what I found. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, this is great! I'm happy that we are moving forward with this because we really need to have something at least.
I left some suggestions with ideas and questions as well. I think we are going in a great direction here 👍🏼
I'm sorry. I know all of this is confusing --we are in the middle of a transition to addons and new dashboard as well at the same time 🙃 and both are pretty big and important changes.
This is great feedback, super helpful! I've fixed most of the small stuff, there are a few outstanding |
Not done yet I think, but we're getting there. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great.
I left some suggestions and some small changes about moving content from the new notifications page to the existing versions page.
Apart from that, I think this PR is ready to merge 👍🏼 . I'm pinging @ericholscher just in case he want to take a look at these changes as well; but I think we are ready to move forward after those changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of great improvement here 💯
OK folks, I think I've addressed all feedback with this last commit. @humitos one last 👀 and then ! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent! 💯
It seems there is a checks error:
Once that's fixed, it will auto merge. It's missing just a small newline 😄 |
🎉 |
Address #11425
This is how I'm planning to handle the list of Addons. Add minimal changes to existing docs (I'm expecting some addons to need more than this) and link to them centrally from
/addons
.How do we feel about this?
📚 Documentation previews 📚
docs
): https://docs--11444.org.readthedocs.build/en/11444/dev
): https://dev--11444.org.readthedocs.build/en/11444/